Skip to content

Add search and skin tones to profile emoji picker - #4910

Open
klopez4212 wants to merge 2 commits into
mainfrom
kennylopez-profile-emoji-picker
Open

Add search and skin tones to profile emoji picker#4910
klopez4212 wants to merge 2 commits into
mainfrom
kennylopez-profile-emoji-picker

Conversation

@klopez4212

Copy link
Copy Markdown
Contributor

Summary

  • Add search and skin-tone controls to profile emoji avatars.
  • Match member-search styling, increase the picker height, and remove the inner skin-tone outline.
  • Cover profile and onboarding pickers with focused E2E coverage.

Testing

  • pnpm build:e2e
  • Focused profile and onboarding Playwright tests
  • Pre-push desktop checks

Signed-off-by: kenny lopez <klopez4212@gmail.com>
klopez4212 added a commit that referenced this pull request Aug 5, 2026
@klopez4212

Copy link
Copy Markdown
Contributor Author

🤖 Screenshots of the profile emoji-picker update.

Profile

Search and the default skin-tone selector sit at the top of the taller picker.

profile-picker

Onboarding

The same controls are available during avatar onboarding.

onboarding-picker

@klopez4212
klopez4212 marked this pull request as ready for review August 5, 2026 15:48
@klopez4212
klopez4212 requested a review from a team as a code owner August 5, 2026 15:48
@klopez4212

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: c1c88ca623

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

The new picker assertions read overridden properties (48px control heights,
the resting search background) immediately after the search input becomes
visible. The Buzz stylesheet is appended from a `requestAnimationFrame` loop
that only starts once `em-emoji-picker` has attached its shadow root, so it
can land a frame or more later. An instrumented probe caught that gap: on 2
of 5 runs the shadow root had no `#buzz-emoji-mart-style` at that read point
and the input measured Emoji Mart's un-overridden 36px instead of 48px.

The shipped tests passed only because incidental round-trips bought enough
time. `expect(...).toBe(48)` does not retry, so the race was live.

Add `expectEmojiMartStylesInstalled` to the CSS helpers, polling for the
style element, and gate both specs on it before their un-retried reads. The
install loop itself predates this branch and is unchanged.

Also documents the second probe finding, which needed no fix: the resting
background samples as transparent right after install because Emoji Mart puts
a 225ms `background-color` transition on the input. It settles to
`rgb(229, 229, 230)`, and the override does win the cascade over the vendor
`var(--em-color-border)` rule.

Co-authored-by: kenny lopez <klopez4212@gmail.com>
Signed-off-by: kenny lopez <klopez4212@gmail.com>
@klopez4212

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1f8368d661

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread desktop/tests/e2e/onboarding-avatar-skip.spec.ts

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing on Wes's behalf. The search/tone functionality and style synchronization look sound, but the increased non-modal picker height introduces a visible onboarding layout regression at the viewport shown in this PR's own screenshot. Please resolve the picker/CTA collision and add a layout assertion before landing.

className="buzz-emoji-mart relative z-0 h-[316px] overflow-hidden rounded-xl bg-muted transition-colors duration-[250ms] ease-out"
className={cn(
"buzz-emoji-mart relative z-0 overflow-hidden rounded-xl bg-muted transition-colors duration-[250ms] ease-out",
isOnboardingModal ? "h-[316px]" : "h-[384px]",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — Keep the onboarding CTA clear of the taller picker. AvatarStep uses the default presentation, so this branch increases its picker from 316px to 384px as well as adding the 68px search row. At the 1280×720 viewport in the PR's own onboarding screenshot, the picker ends around y=562 while the Next button starts around y=558; the picker visibly paints over the top of the CTA. That is a user-facing regression on a supported desktop-sized viewport, and the new E2E test currently locks in 384px without asserting that the actions remain visible and non-overlapping. Please make the height/layout responsive (or give this onboarding surface an appropriate presentation-specific height) and add a bounding-box/non-overlap assertion for the actions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants